Skip to content

Clarify private key permissions in Set-DbaNetworkCertificate help - #10586

Merged
potatoqualitee merged 1 commit into
developmentfrom
clarify-network-certificate-key-permissions
Aug 20, 2026
Merged

Clarify private key permissions in Set-DbaNetworkCertificate help#10586
potatoqualitee merged 1 commit into
developmentfrom
clarify-network-certificate-key-permissions

Conversation

@andreasjordan

Copy link
Copy Markdown
Collaborator

Documentation-only fix for the wording that caused the confusion in #10125.

Problem

The comment-based help said:

This command also grants read permissions for the service account on the certificate's private key.

Readers reasonably take "the service account" to mean the startname of the service. That is not what the code does, and @rjleue hit exactly that in #10125 — their SCOM monitoring flagged the instance because the expected ACE was not on the startname.

Mechanism

The code grants Read on the private key file to the per-service SID of the instance — NT SERVICE\MSSQLSERVER for the default instance, NT SERVICE\MSSQL$<InstanceName> for named instances (Set-DbaNetworkCertificate.ps1#L193-L206):

# Grant permissions to the Service SID
$sqlSSID = "NT SERVICE\MSSQLSERVER"
if ($instance.InstanceName -ne "MSSQLSERVER") {
    $sqlSSID = "NT SERVICE\MSSQL$" + $instance.InstanceName
}

The service SID is always part of the process token of the SQL Server service, so this grants the running instance exactly the access Microsoft's docs ask for, and it survives a change of the startup account. With the default virtual account the two are literally the same object; the difference only shows up with a domain account or a gMSA.

So the behaviour is correct — the help was not.

What changed

Two blocks of comment-based help in public/Set-DbaNetworkCertificate.ps1:

  1. .DESCRIPTION — now names the per-service SID explicitly and explains why it is used instead of the startname.
  2. .PARAMETER Thumbprint — dropped the "the SQL Server service account will be granted read permissions to it" clause and points at the description.

What deliberately did not change

  • No code. The begin/process blocks are untouched; the diff is entirely inside the help block.
  • .OUTPUTS ServiceAccount still reads "The service account running the SQL Server instance". That property comes from $wmiService.ServiceAccount and really is the startname, so the wording is accurate there.

Sweep for the same wording elsewhere

Set-DbaNetworkCertificate is the only command in the repo that ACLs a certificate private key — NT SERVICE\MSSQL appears nowhere else in public/ or private/ outside of this command. I checked the sibling network-certificate commands and the computer-certificate family:

  • Get-DbaNetworkCertificate, Test-DbaNetworkCertificate, New-DbaComputerCertificate, Add-DbaComputerCertificate, Backup-DbaComputerCertificate — none of them claim to grant private key permissions, so there is nothing to correct.
  • Remove-DbaNetworkCertificate mentions ServiceAccount only as an output property, same accurate usage as above.

No other help text needed changing.

Testing

Documentation-only, so no behavioural test applies. Verified that the file parses cleanly with [System.Management.Automation.Language.Parser]::ParseFile and that the help block still renders as intended via Get-Help -Full, including the literal MSSQL$<InstanceName> surviving PowerShell's parsing of the comment block.

Reported by @rjleue in #10125.

…private key

The help said the command grants read permissions "for the service account",
but the code grants them to the per-service SID of the instance
(NT SERVICE\MSSQLSERVER, or NT SERVICE\MSSQL$<InstanceName> for named
instances). Readers assumed the startname of the service was used, which
caused confusion in the issue.

Documentation only - no code changed. The .OUTPUTS ServiceAccount property
still documents the startname reported by WMI, which is correct as written.

Refs #10125

(do Set-DbaNetworkCertificate)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@potatoqualitee

Copy link
Copy Markdown
Member

thank you 🙏🏼

@potatoqualitee
potatoqualitee merged commit 117e261 into development Aug 20, 2026
21 checks passed
@potatoqualitee
potatoqualitee deleted the clarify-network-certificate-key-permissions branch August 20, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants